Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support of kerning to Text #537

Merged
merged 3 commits into from
Mar 18, 2019
Merged

Added support of kerning to Text #537

merged 3 commits into from
Mar 18, 2019

Conversation

K-Be
Copy link
Contributor

@K-Be K-Be commented Mar 1, 2019

Added support of kerning to Text.
Faced with a strange problem with kerning and phrase "Hello, world", but in other cases kerning has correct behaviour. I think, it is a bug of NSAttributedString.

@ystrot ystrot self-assigned this Mar 1, 2019
@ystrot ystrot added this to the 0.9.5 milestone Mar 1, 2019
@ystrot
Copy link
Member

ystrot commented Mar 1, 2019

Hi Andrew,

Thank you for your work! I'll review it and merge in the beginning of the next week.

@K-Be
Copy link
Contributor Author

K-Be commented Mar 4, 2019

What do think about make kerning property an optional property?

@ystrot
Copy link
Member

ystrot commented Mar 4, 2019

I was thinking about this, however logically no kerning equals to kerning = 0, so I think zero value is fine.

@K-Be
Copy link
Contributor Author

K-Be commented Mar 5, 2019

With and without 0 the kerning has different behaviour (look at link that I've added to my PR).
I do not add Kern attribute to NSAttributedString if kerning is equal 0.

@ystrot
Copy link
Member

ystrot commented Mar 18, 2019

Hi Andrew,

I agree, no matter how it works, logically zero kerning means no kerning and we don't need to fix API to match current implementation. So your workaround is fine. I'm going to merge your patch now. Thank you for your work!

@ystrot ystrot merged commit 9124c94 into exyte:master Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants